Skip to content

[MISC] Fix gltf Loading for URDF #1857

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Oct 22, 2025
Merged

[MISC] Fix gltf Loading for URDF #1857

merged 6 commits into from
Oct 22, 2025

Conversation

@ACMLCZH
Copy link
Contributor

@ACMLCZH ACMLCZH commented Oct 13, 2025 •
edited by duburcqa
Loading

When URDF loads gltf meshes, it uses "y-up" by default, but it currently loaded as "z-up" now.
franka_1

Resolves #1820

@YilingQiao
Copy link
Collaborator

YilingQiao commented Oct 13, 2025

can you add a unit test? Just for this scene and compare the image pixel?

@duburcqa
Copy link
Collaborator

duburcqa commented Oct 13, 2025

can you add a unit test? Just for this scene and compare the image pixel? Pixel machine may not be the best, because we don't really look at them :/ We about computing the principal axes and check if it matches the one for the collision geometry?

@YilingQiao
Copy link
Collaborator

YilingQiao commented Oct 13, 2025 •
edited
Loading

how about computing the principal axes and check if it matches the one for the collision geometry?

how to define the principal axes? Maybe we can find the longest dimension in an object. I don't have a strong opinion though.

@duburcqa
Copy link
Collaborator

duburcqa commented Oct 13, 2025 •
edited
Loading

how to define the principal axes?

You ask trimesh to do it for you :)

_components, vectors = trimesh.inertia.principal_axis(tmesh.moment_inertia)

YilingQiao
YilingQiao reviewed Oct 14, 2025
View reviewed changes
@YilingQiao YilingQiao merged commit 348ac3a into Genesis-Embodied-AI:main Oct 22, 2025
19 checks passed
Milotrince
Milotrince reviewed Nov 1, 2025
View reviewed changes
genesis/ext/urdfpy/utils.py
Copy link
Contributor

@Milotrince Milotrince Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change causes allegro_hand_right_glb.urdf to be loaded with flipped axes. I see that there is the parse_glb_with_zup option in gs.morphs.Mesh but no such option seems to be exposed for gs.morphs.URDF

Copy link
Collaborator

@YilingQiao YilingQiao Nov 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Is this PR to fix Allegro hand? There are two versions of this hand?
#1820

Copy link
Contributor Author

@ACMLCZH ACMLCZH Nov 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add an option for URDF: #1938

Copy link
Contributor

@Milotrince Milotrince Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah seems that there are different versions floating around. There is no "fixing" the hand, just need to make it clear know what the default convention is and provide an option to load it with different axes.
Thanks @ACMLCZH for the quick PR

@ACMLCZH ACMLCZH mentioned this pull request Nov 2, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@YilingQiao YilingQiao YilingQiao approved these changes

@duburcqa duburcqa Awaiting requested review from duburcqa duburcqa is a code owner

+1 more reviewer

@Milotrince Milotrince Milotrince left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Issues loading Allegro hand URDF model

4 participants

@ACMLCZH @YilingQiao @duburcqa @Milotrince